Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

websocket: avoid WebSocketClosedError double/nested exception #2926

Closed

Conversation

ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Sep 30, 2020

Tracking the exception that was being handled when another exception
was raised is a new feature in Python-3, causing this:

Traceback (most recent call last):
  File "/...-packages/tornado/websocket.py", line 1104, in wrapper
    await fut
tornado.iostream.StreamClosedError: Stream is closed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/...-packages/tornado/websocket.py", line 1106, in wrapper
    raise WebSocketClosedError()
tornado.websocket.WebSocketClosedError

This can be selectively avoided using the new Python-3 syntax:

raise WebSocketClosedException from None

I just find the double-exception to be more ugly than it has to be, a matter of taste I guess, so I humbly offer this easy change in case you find it acceptable.

Tracking the exception that was being handled when another exception
was raised is a new feature in Python-3, causing this:

    Traceback (most recent call last):
      File "/...-packages/tornado/websocket.py", line 1104, in wrapper
        await fut
    tornado.iostream.StreamClosedError: Stream is closed

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/...-packages/tornado/websocket.py", line 1106, in wrapper
        raise WebSocketClosedError()
    tornado.websocket.WebSocketClosedError

This can be selectively avoided using the new Python-3 syntax:

    raise WebSocketClosedException from None
@bdarnell
Copy link
Member

bdarnell commented Nov 3, 2020

As I said in #2824 (comment), I'm generally not a fan of from None - it loses information about where exactly in self._write_frame the exception was raised. Maybe that's rarely useful, but for the rare cases where it matters it seems worth the occasional doubled stack trace. (on the other hand, preventing stack traces from being logged at all for closed connections is generallly a good thing).

@bdarnell bdarnell closed this Nov 3, 2020
@ploxiln ploxiln deleted the websocket_closed_from_none branch May 29, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants